Add Smctr/Ssctr YAML and properly attribute SCTRCLR to them #554
Conversation
AFOliveira
left a comment
AFOliveira
left a comment
There was a problem hiding this comment.
I think you did not add the CSRs that this extension comprehends, should we have an issue to track that?
Other than that LGTM
arch/ext/Smctr.yaml
Outdated
| implies: | ||
| - if: S | ||
| then: | ||
| name: Ssctr | ||
| version: "1.0.0" |
There was a problem hiding this comment.
Does this statement from the spec need to be accommodated here, and similarly in the "Ssctr" definition below?
Smctr and Ssctr depend on both the implementation of S-mode and the Sscsrind extension.
...or maybe we just need a "requires" field here?
requires: [ S, Sscsrind ]
There was a problem hiding this comment.
"M" is required here, too, I believe.
There was a problem hiding this comment.
You are right, we need the 'requires' line you propose. Given that, the condition in the implies line isn't needed -- S must be implemented if Smctr is.
For the "M" requirement: you would think so ;) But, if that's the case, there is no need for Ssctr at all. Now, what you do with a system that has S but no M is a question I can't answer...
How should we define the "Entry Registers" from Chapter 3 of the extension spec? Maybe they'll come when "Sscsrind" is defined, since access to the registers seems indirect through "siselect/sireg"? |
Yes, I'll add an issue for both Sscdrind and the CSRs of Smctr/Ssctr. |
|
@dbrash, see comments above |
|
If we adopt #598, we can tag the non-normative part of description to mark it as "non-ratified" |
arch/inst/Smdbltrp/sctrclr.yaml
Outdated
| * Zeroes all CTR Entry Registers, for all DEPTH values | ||
| * Reset to Zero the optional CTR cycle counter where implemented | ||
| ** `ctrdata.CC` and `ctrdata.CC` bit fields. |
There was a problem hiding this comment.
@dbrash : this is the same field mentioned twice?
|
Apologies, it should have been ctrdata.cc and ctrdata.ccv fields. //section
11.5.3, Smctr chapter privileged ISA spec.
+ * Zeroes all CTR Entry Registers, for all DEPTH values
+ * Reset to Zero the optional CTR cycle counter where implemented
+ ** `ctrdata.CC` and `ctrdata.CCV` bit fields.
@dbrash <https://github.com/dbrash> : this is the same field mentioned
twice?
…On Thu, Apr 17, 2025 at 7:17 PM Derek Hower ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In arch/inst/Smdbltrp/sctrclr.yaml
<#554 (comment)>
:
> @@ -3,10 +3,42 @@
$schema: inst_schema.json#
kind: instruction
name: sctrclr
-long_name: No synopsis available.
-description: |
- No description available.
-definedBy: Smdbltrp
+long_name: Supervisor Control Transfer Record (CTR) clear
+description:
+ - id: inst-sctrclr-operation
+ normative: false
+ text: |
+ When `mstateen0.CTR`=1, the SCTRCLR instruction performs the following operations:
+
+ * Zeroes all CTR Entry Registers, for all DEPTH values
+ * Reset to Zero the optional CTR cycle counter where implemented
+ ** `ctrdata.CC` and `ctrdata.CC` bit fields.
@dbrash <https://github.com/dbrash> : this is the same field mentioned
twice?
—
Reply to this email directly, view it on GitHub
<#554 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BH75UD533ITZVEEZ7SW467D2Z7V2FAVCNFSM6AAAAABZ5LFAT6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDONZWGU4DCNBTGI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***
com>
|
Co-authored-by: David Brash <[email protected]>
Signed-off-by: Derek Hower <[email protected]>
Signed-off-by: Derek Hower <[email protected]>
ThinkOpenly
left a comment
ThinkOpenly
left a comment
There was a problem hiding this comment.
I think this just needs a rebase. Paths have changed.
|
The changes in this PR were copied to #1238, which has now been merged. Closing this PR. |
Pull request was closed
No description provided.